Conversation
…ninstall, improved description for interactive and non-interactive modes, structure of folder with measurement results, Contributing, FAQ
There was a problem hiding this comment.
Pull request overview
This PR aims to improve JCAlgTest usability and repository clarity by expanding user-facing documentation/help, introducing a console verbosity flag, hardening performance-test recovery behavior, and updating project/repo housekeeping files.
Changes:
- Expanded README/help guidance (examples, troubleshooting, contribution instructions) and added several new reference docs under
docs/. - Added a
-verboseCLI flag and extended logging/error-handling paths for performance testing. - Updated NetBeans/Ant project metadata and repo ignore rules as part of cleanup.
Reviewed changes
Copilot reviewed 17 out of 20 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/smartcard_shops.txt | Adds a short list of smartcard vendor links. |
| docs/card_identification.txt | Adds reference links for identifying cards (NXP IDENTIFY, tooling sources). |
| docs/RID_list.txt | Adds a large RID-to-applicant mapping list (data/reference). |
| docs/DESFireSupportReadme.md | Adds extensive DESFire/Chameleon Mini documentation and usage examples. |
| coverity_model | Removes a previously-empty Coverity model stub. |
| README.md | Major usability/documentation expansion (modes, prerequisites, FAQ, contribution flow). |
| AlgTest_Process/run.bat | Changes output paths for generated web artifacts. |
| AlgTest_Process/nbproject/project.properties | Updates build classpath references for AlgTestProcess. |
| AlgTest_Process/cplc.py | Extends IC type mapping and removes duplicated logic. |
| AlgTest_JClient/src/algtestjclient/PerformanceTesting.java | Improves exception messaging and adds reconnect/retry guidance during perf tests. |
| AlgTest_JClient/src/algtestjclient/DirtyLogger.java | Adds “detail” logging methods and a verbose flag on the logger. |
| AlgTest_JClient/src/algtestjclient/Args.java | Introduces -verbose CLI flag. |
| AlgTest_JClient/src/algtestjclient/AlgTestJClient.java | Adds richer --help output and a top-level error banner; wires verbose into logger construction. |
| AlgTest_JClient/nbproject/project.xml | Adds a NetBeans build extension referencing build-native.xml. |
| AlgTest_JClient/nbproject/project.properties | Updates jcardsim jar reference and enables native bundling (and tweaks classpath). |
| AlgTest_JClient/nbproject/genfiles.properties | Updates generated CRCs for NetBeans build files. |
| AlgTest_JClient/nbproject/build-impl.xml | Imports build-native.xml into the generated Ant build. |
| .gitignore | Adds additional ignored paths (e.g., heap/, client logs, store output). |
Comments suppressed due to low confidence (1)
AlgTest_JClient/src/algtestjclient/DirtyLogger.java:67
m_verboseis set via the new constructor, but the regularprint/printlnmethods ignore it, andprintlnDetail/printDetailaren’t used anywhere else in the codebase. As a result, the-verboseflag currently has no effect on console verbosity. Either route the noisy per-algorithm output through the*Detailmethods, or haveprint/printlnsupport log levels so the flag actually changes what is printed to stdout.
boolean m_verbose = false;
public DirtyLogger(FileOutputStream logFile, boolean bOutputSystemOut) {
m_logFile = logFile;
m_bOutputSystemOut = bOutputSystemOut;
}
public DirtyLogger(FileOutputStream logFile, boolean bOutputSystemOut, boolean verbose) {
m_logFile = logFile;
m_bOutputSystemOut = bOutputSystemOut;
m_verbose = verbose;
}
public void println() {
String logLine = "\n";
print(logLine);
}
public void println(String logLine) {
logLine += "\n";
print(logLine);
}
public void print(String logLine) {
if (m_bOutputSystemOut) {
System.out.print(logLine);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Overall, the introduced in the PR changes are nicely addressing the found usability issues. Upon addressing those Copilot's suggestions, could be merged |
Add -pre-compile target in AlgTest_Process/build.xml that builds AlgTest_JClient first, ensuring its dist jar exists before compilation.
xhanulik
changed the title
Collaborator
|
@petrs could you please test the build without the netbeans project files? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces several improvements to the AlgTest JavaCard testing tools, focusing on enhanced usability, error handling, and support for new smartcard profiles. The most significant changes are:
More detailed description is provided below.
Usability and Documentation Improvements:
AlgTestJClient, including usage examples, common troubleshooting tips, and bug reporting instructions. This is shown when the-helpflag is used. [1] [2]Logging and Output Enhancements:
-verbosecommand-line flag to control console output detail; detailed per-algorithm logs are now shown on the console only when verbose mode is enabled, but always written to the log file. [1] [2] [3] [4]Error Handling and Recovery: